-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feat/metrics controller #23
Conversation
ZKS-60 Metrics controller
Implement Metrics defined in Notion tech-design doc. And make them retrieve mock data |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we include natspec for all Modules, Services, Interfaces, DTO's? Is something that we are missing and is very important
apps/api/test/metrics.e2e-spec.ts
Outdated
afterAll(async () => { | ||
await app.close(); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any performance reason, or something like that, for this not to be an afterEach
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice catch 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GJ! Wdyt about removing NatSpec for constructor methods? At least for DTOs, I noticed that we are not going to have complex logic there, so I find it a bit redundant to have NatSpec for that.
@@ -3,10 +3,11 @@ import { ProvidersModule } from "@packages/providers"; | |||
|
|||
import { ApiController } from "./api.controller"; | |||
import { RequestLoggerMiddleware } from "./common/middleware/request.middleware"; | |||
import { MetricsController } from "./metrics/metrics.controller"; | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
natspec missing here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
on this .module you say we add the Natspec due to the consumer applied? because in general .module doesn't have anything to be documented
🤖 Linear
Closes ZKS-60 ZKS-61
Description
Adds Metrics controller with eps:
Both return mocked data